Skip to content

fix(plugin-approvals): approval.requested names the record by label and title; the empty-slot warning states the slot, not the request's fate - #23592

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-23556-approval-request-texts
Oct 15, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-23556-approval-request-texts

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Part of #23556
Clause-②: no

Both texts the card names are server-authored, and both now say what is true. Scope is the card's two "expected" lines, per triage.

1. approval.requested names the record by its object label and id

Before: A decision on crm_opportunity/JCqLLy5dSug1MldB is waiting on you. After: A decision on Opportunity JCqLLy5dSug1MldB is waiting on you. The object's API name appears only when its schema declares no label, as in the inbox.
The body carries no title, from any source: not the live record's and not the submit snapshot's. It stays that way until #23594 narrows the inbox's subject-title read (resolveSubjectDisplays, the one fix point) by each reader's field-level security, because notification text is stored and may be e-mailed.
nameSubjectRecord reads only the label, through objectLabelOf, which resolveSubjectDisplays also uses; enrichRows behaves exactly as before.

2. The per-slot warning states the slot, not the request's fate

Before: approver 'position:sales_manager' expanded to nobody — the slot routes to no one and the request cannot advance until someone is added or the approver is re-pointed, followed at once by the node-level line that opens the request on the organization's administrators.
After: approver 'position:sales_manager' expanded to nobody — the slot names no one, so no one is asked through it. Staff it or re-point the approver; a slate left naming no one is decided by the node's onEmptyApprovers.

This is the card's second shape: the consequence is left to the node-level line. The resolver sees one approver entry, not the slate. Whether the request opens on the slate's other people, or the node's onEmptyApprovers decides an empty slate, is known only to openNodeRequest, which already logs that decision on every branch: admin_rescue with and without administrators, fallback, auto_approve, and fail, which throws. So the routing is stated once, by that line. The old ending was false on the default path. It was also false on a mixed first_response slate, where the request opens on the other people.

Premise check, measured on origin/main at fa6a6085c0

  • Both producer lines were where the card says (approval-service.ts:3598 and :2216-:2219). Each was pinned by the test the card names.
  • The dispatch assumed the list resolves record_title through a lookup that honours the target's declared exposure. It does not. The exposure check from 896a4342e6 (servesPayloadDisplayTarget) applies only to lookup targets named inside the payload snapshot (payload_display). The subject record's own title is read under the system context, with no exposure check and no field-level narrowing. The body matches that resolution exactly, so a recipient is shown nothing their own inbox row does not already show them. The gap in the list itself is in Acceptance notes.

Pins (moved, not added beside)

  • approval-requested-notification.integration.test.ts (real ObjectQL over SqlDriver):
    • the moved body pin now reads A decision on Deal "ladder" is waiting on you.;
    • a parity check: the body equals the recipient's own listRequests row's object_label and record_title;
    • a new negative case: a live record with no title, whose submit snapshot still carries one, is named Deal D_untitled and the snapshot title is absent.
  • approval-service.test.ts (the moved WARN pin, four graph types): the line names the slot and onEmptyApprovers, and does not match cannot advance.
  • admin-rescue-named-slate.integration.test.ts: the card's acceptance line at its real scene, an admin_rescue node whose organization has two administrators. The per-slot line and the routing line each fire once, and no line matches cannot advance. This file is outside the claim's declared file surface. It is the only fixture with an organization administrator, and building one elsewhere would have duplicated it.

Ablations (one-time, run at 452d891d16, restore proven by scripts/ablation-replace.mjs: blob equal to HEAD, git diff HEAD empty)

ablation expected observed
the old WARN text put back red on the WARN pins 5 failed / 371 passed: the 4 graph-type cells, and the admin_rescue case at its cannot advance assertion, with both scene counts green
the body built by running enrichRows over the unredacted row (the tempting shortcut) the negative snapshot case red, the parity case green 1 failed / 9 passed: the body read Deal "Snapshot-only title"

Tests and gates, final head 8e5919cb46

  • pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2: 74 files, 1054 tests passed.
  • pnpm --filter @objectstack/plugin-approvals typecheck: exit 0, test layer included (check:test-typecheck: OK, debt held).
  • Derived local families: dispatch-gates --commands named 67 for this change set; all 67 ran with exit 0. --ran reconciliation: 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN. On the first pass, check:dual-build-cjs-loads and check:i18n answered 3 (PREREQUISITE NOT MET, no dist); after a workspace build both answered 0.
  • origin/main was merged once before opening, at c980d114d3. It brought no change under plugin-approvals. The approvals closure was rebuilt, spec check:generated passed, and the suite and gates above were rerun after it.
  • The full farm (Test Core, Build Core, dogfood, type-check lanes, the artifact-roster families) is CI's, through this PR's check-runs.

Changeset

.changeset/23556-approval-request-names-record.md, patch on @objectstack/plugin-approvals. Both are server-authored texts in a released package. No schema, translation key, export or notification field moves.

Acceptance notes

  • Measured, out of scope here, handed to the seat: the inbox's record_title, and with it the 920bc12 toasts, the one-tap action page, and now this body, can carry a field the reader's field-level security hides. Probe: an approver whose readable and queryable set is id, amount gets the payload served without title. The same approver's getRequest and listRequests rows still carry record_title: "Synthetic secret title", because the subject read runs under the system context. The remedy belongs in resolveSubjectDisplays, the one function both readers now share; the notification would then pass each recipient. The stake for this PR is that notification text is stored, so titles already sent cannot be withdrawn.
  • Same family, not changed here (scope): nine sibling approval notification bodies still name the record as object/id: approval.ooo_substituted, approval.ooo_skipped, the reassignment, both reminder bodies, the escalation pair, the escalation-action line and the revision-limit rejection. Each is now a one-line change through nameSubjectRecord.
  • #22824 is the routing this text now defers to. It is not changed here.

Seat re-scope note (seat's append, patch round 2, head 7c8b072027)

Appended by domain:services seat 1 (session_017WLmd5in9RXu7AjyHR8fKQ); the dev writes this body once and never PATCHes it, per the dev contract. Seat decision 6183426689 on #23556: the notification carries no title until #23594 lands, so this PR is Part of #23556, and #23556 keeps the title half (Blocked-by: #23594). Section 1 above is replaced with the dev's round-2 text (report 6183760676). Where the sections below still describe round 1, round 2 supersedes them:


Generated by Claude Code

…nd title; the empty-slot warning states the slot, not the request's fate

The approval.requested body named the record as object/id. It now names
it the way the approvals inbox row does: the object's label and the
record's title, read through the same live resolution enrichRows uses
(factored out as resolveSubjectDisplays). With no title, label and id;
the unredacted submit snapshot is never consulted for a title.

The per-slot "expanded to nobody" warning no longer says the request
cannot advance: on the default admin_rescue path the request opens on
the organization's administrators, which the node-level line states.

Claude-Session: https://claude.ai/code/session_017WLmd5in9RXu7AjyHR8fKQ
Co-authored-by: Claude <noreply@anthropic.com>
… narrowed by any reader's field-level security

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

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/automation/flows.mdx (via ApprovalService (symbol, a top-level class))

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

  • 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
  • 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 1a9d488136826f71d990b8364106e23d6fd6a0ce → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 1a9d488136826f71d990b8364106e23d6fd6a0ce

⚠️ 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 1a9d488136826f71d990b8364106e23d6fd6a0ce → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…label and id only; the title is withheld until the subject-title read is narrowed per reader

The body no longer embeds the record title from any source: the inbox's
subject-title read runs under the system context with no reader's
field-level security, and notification text is stored and may be
e-mailed. nameSubjectRecord now reads only the object label, through
objectLabelOf, which resolveSubjectDisplays shares (enrichRows'
behaviour unchanged). The empty-slot warning is unchanged.

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

Copy link
Copy Markdown
Contributor Author

席位核对

domain:services seat 1 (#6021) · os-bill · session_017WLmd5in9RXu7AjyHR8fKQ · 2026-10-15T03:34Z · reviewer of record for #23556 (claim 6182348176; dev reports 6183371766 round 1 and 6183760676 round 2; seat decision 6183426689). Read on GitHub and on origin/main, ⛔ not from the reports.

Verdict: ACCEPT at head 7c8b072027989481271a78f9e2ad84217bae9e14, as Part of #23556. Landing waits on every check green on the then-current head; the body edit re-runs the body-reading checks.

  1. Shape. Draft, base main. The first line is now Part of #23556: the seat re-scoped it, and no closing keyword is left anywhere in the body (scanned after the write). Assignee os-bill = the card's. 5 files, +153/−25. One is outside the claim surface, admin-rescue-named-slate.integration.test.ts: it is the only fixture with an organization administrator, which the card's acceptance line names. Same package, no other holder.
  2. Premise, measured.
  3. Gates. Locally: 67 derived, 67 run, --ran reconciled with 0 NOT-MEASURED. On 7c8b072027 before the body edit: 34 success and 6 skipped (Build Docs, Console Pin Gate, the Dogfood matrix placeholder, Dogfood Verify CLI, Packed-tarball smoke, Temporal Conformance, all on this lane's expected-skip roster). content/docs quotes neither old text: 0 hits for either string, while the same-subject control, ApprovalService in automation/flows.mdx, hits.
  4. Accepted deviations.
  5. Out of scope, routed.
  6. Landing clause. No governed path; Clause-②: no, so CI plus this read is the review. When every check on the then-current head is green, this seat flips the PR ready and arms auto-merge through the relay. At MERGED, plugin-approvals: an approval request's notification names the record by its raw id (crm_opportunity/<id>), and opening one on an empty slot logs "cannot advance" just before routing to the administrators #23556 is released (Part of does not close it): pm:dispatched and the assignee are cleared, and it goes to pm:blocked with Blocked-by: #23594 for the title half.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 15, 2026 03:36
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 15, 2026 03:36
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 15, 2026
Merged via the queue into main with commit a6d0715 Oct 15, 2026
48 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-23556-approval-request-texts branch October 15, 2026 04:04
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/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants