Skip to content

fix(plugin-approvals): the out-of-office notifications name the absent approver and the delegate by display name - #24146

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-24065-ooo-notification-names
Oct 17, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-24065-ooo-notification-names

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #24065
Clause-②: no

What changed

The out-of-office fan-out in openNodeRequest now names both people by display name in its two notification bodies. Path: packages/plugins/plugin-approvals/src/approval-service.ts, line numbers at this head.

  • approval.ooo_substituted (:3640) goes to the delegate and names the absent approver. You are covering an approval on Deal D_ooo while u_ooo is out of office. now reads … while Olive Away is out of office.
  • approval.ooo_skipped (:3652) goes to the absent approver and names the delegate. An approval on Deal D_ooo was routed to u_bob while you are out of office. now reads … was routed to Bob while you are out of office.

Both names come from resolveUserNames (:6680), the plugin's one person-label chain. The approvals door uses the same resolver for pending_approver_names and for the names on the timeline.

  • The fan-out calls it once, at :3622, before the loop, with both people (sub.from and sub.to) of every hop.
  • The chain removes duplicates, so an A → B → C chain is one sys_user read for three people.
  • With no substitution, the call has no targets and makes no read: readUserNames returns before its find.

No resolver was added. resolveUserNames itself, its fallback and its field-level-security note are unchanged. The record half (nameSubjectRecord, from #23978) is untouched.

What the body shows when there is no name, and whether that can be an email

sub.from and sub.to are the hop's delegator_id and delegate_id (applyOooDelegation). Both are sys_user lookups, so the stored value is a user id, never an email. For each person, the body shows:

what the chain reads the body names the person as
an account row with a name the name (Olive Away)
an account row with no name but with an email the email address. readUserNames labels such a row with u.name ?? u.email. This is the same label the approvals door already serves for that account, as pending_approver_names and on the timeline. sys_user.name is declared required: true (packages/platform-objects/src/identity/sys-user.object.ts), so only a row written without a name reaches this case.
no account row for the id, or a failed sys_user read the stored user id, which is the text the body carries today. This is the rule the spec's ApprovalActionRow states for every reader of this chain: when no name is returned, "render the address".

Each body goes to one party of the hop's delegation and names the other party.

A body is a stored copy of the name: messaging stores it and the inbox serves it. The resolveUserNames docblock warns, under "STORED COPIES", that a later narrowing of names by sys_user field-level security does not reach the copies stored in rows. It lists two columns, and these two bodies are a third copy. The comment at the call site says so. Adding the bodies to the docblock's list is outside this claim's file surface, so it is an open question in the report (see Acceptance notes).

The audit comment keeps the ids, by design

The ooo_substitute row's comment (:3629, for example u_ooo → u_bob — leave) is unchanged. These read it:

  • The approvals door. listActions serves the comment as stored. It resolves names per read for actor_id, reassign_from and reassign_to. This row has no person in any of those fields (actor_id is null).
  • The console. The request timeline renders a.comment verbatim, in quotes: ApprovalsInboxPage.tsx and RecordApprovalsPanel.tsx at the console pin f8267df93a.
  • The list views. sys_approval_action's recent and all_actions list views show the column, and an export of the object carries it.
  • The platform checklist. The OOO item in docs/qa/platform-checklist/areas/approvals.json verifies the row by its comment reading A → B, the two identities.

This comment is the request's audit trail of identities. An id stays true when a user is renamed. A name written into this row would be one more copy that a later field-level-security narrowing cannot reach. Where the platform wanted a hand-off's parties to be readable, it chose structured fields with names resolved at read time: reassign_from / reassign_to and their *_name companions replaced the "FROM → TO" text that reassign used to write into its comment (spec ApprovalActionRow). A readable OOO timeline line is a job for that shape, not for this comment (see Acceptance notes).

Premise check (origin/main 27d7c2aa1d)

The premise holds.

  • The bodies at :3624 and :3636 interpolated sub.from and sub.to as raw ids.
  • The audit comment is at :3613, and resolveUserNames is at :6664.
  • notify() passes payload.body to messaging.emit unchanged. The red run below shows the bodies as messaging received them.

Pin

approval-notification-body.integration.test.ts is the battery that drives every call site that notifies. It runs the real ApprovalService over a real ObjectQL on SqlDriver (better-sqlite3, in memory), and records what reaches the messaging surface. This PR adds one request and two tests:

  • Request ooo_gone is the control. Its approver u_gone has no sys_user row and an active delegation to u_alice. So in one fan-out, one person's name resolves and the other's cannot.
  • Test: both OOO bodies name people by display name.
    • It asserts the person's name in each body: Olive Away in ooo_substituted@ooo, Bob in ooo_skipped@ooo, and Alice in ooo_skipped@ooo_gone. It also asserts that the matching id is absent. It does not assert the rest of the sentence.
    • It then sweeps every OOO body the battery sent for any of the battery's user ids.
  • Test: the fallback. The ooo_substituted@ooo_gone body names u_gone, so the slot holds the id, not undefined and not an empty string.

The battery's coverage list gains the two ooo_gone sites.

Readings:

  • Red against the unfixed source (test-only commit 225281a720). One test failed and six passed. The three offenders were the bodies as sent: You are covering an approval on Deal D_ooo while u_ooo is out of office., An approval on Deal D_ooo was routed to u_bob while you are out of office. and An approval on Deal D_ooo_gone was routed to u_alice while you are out of office. The fallback pin was green before the fix, because the id was the whole text then.
  • Green at 26ca62ac57: the battery passes 7/7.
  • Ablation 1 (scripts/ablation-replace.mjs, WRAP mode): oooPerson returns the bare id, which puts the old text back in both bodies. The anchor went from 1 hit to 0 and the blob from 05d5b39c94e8 to 269142866144. The name pin went red with exactly the three offenders above, and the other six tests stayed green. The restore was proven: the blob is 05d5b39c94e8, equal to HEAD, and git diff HEAD is empty.
  • Ablation 2: oooPerson returns the chain's answer with no fallback. The blob went from 05d5b39c94e8 to c88096bc793b. The fallback pin went red with You are covering an approval on Deal D_ooo_gone while undefined is out of office., and the other six tests stayed green. The restore was proven: the blob is 05d5b39c94e8, equal to HEAD, and git diff HEAD is empty.
  • The test imports the subject relatively from src/, so neither ablation has a dist/ leg.

Tests and gates (head de3023f846, after merging origin/main 462d02f00a)

  • Package tests: pnpm --filter @objectstack/plugin-approvals exec vitest run --maxWorkers=2 passed 81 files / 1107 tests.
  • Typecheck: pnpm --filter @objectstack/plugin-approvals typecheck exits 0. check:test-typecheck holds the ledger unchanged at 8 files / 324 errors.
  • Derived gates: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 67 commands. I ran all 67 and recorded each exit before any pipe. 65 exited 0 on the first pass.
    • Two exited 3 (PREREQUISITE NOT MET): pnpm check:dual-build-cjs-loads (some package had no dist/) and pnpm check:i18n (the CLI build closure was missing).
    • I built both prerequisites with the commands each gate prints. Turbo answered from cache: 61/61 cached for the i18n closure, and 75/76 for the whole workspace.
    • On re-run, both exit 0. check:i18n reports all 10 packages' bundles in sync. check:dual-build-cjs-loads reports 111 require entry points across 69 packages load.
  • dispatch-gates --ran over the final record: 67 derived, 67 run, 0 NOT-MEASURED, 0 UNRUN, exit 0. Over the first-pass record, it reports 65 run and 2 NOT-MEASURED, derived from the two exit 3 codes.
  • Changeset level axis, measured offline: node scripts/check-changeset-no-major.mjs --base origin/main --event with this body exits 0 (patch under Clause-②: no).
  • CI-run families: this PR's check-runs are the reading.

Acceptance notes

  • The resolveUserNames "STORED COPIES" docblock lists pending_approver_labels and acted_as_label as the stored copies that a field-level-security narrowing must follow. These two bodies are now a third. The call site records this, and the docblock does not. The claim excluded resolveUserNames itself, so the one-line addition is left to the seat.
  • The OOO row on the console timeline. At the console pin, the timeline maps no label for ooo_substitute, so it shows the raw kind ooo_substitute, followed by the comment "u_ooo → u_bob — leave". This is a reading of the source only, not measured in a running console. The structured-parties shape that reassign uses is the fix pattern that preserves the audit comment. Carrier: none.
  • A delegation chain. In an A → B → C chain, B is told they are covering for A, although B is also away and the slot went on to C. That is the existing per-hop design, and this PR does not change it.

Generated by Claude Code

The battery's out-of-office request now asserts that the delegate's body
names the absent approver, and the absent approver's body names the
delegate, by display name and never by sys_user id. A control request routes
an approver id no account answers to: its body keeps that id.

Red against the unfixed source: both bodies of the `ooo` hop and the skipped
body of the control still carry the raw ids.

Claude-Session: https://claude.ai/code/session_018VqJPGA1NDNSCZsnecsChP
Co-authored-by: Claude <noreply@anthropic.com>
…t approver and the delegate by display name

`openNodeRequest`'s out-of-office fan-out interpolated `sub.from` and
`sub.to` as raw sys_user ids into the `approval.ooo_substituted` and
`approval.ooo_skipped` bodies. Both bodies now name each person through
`resolveUserNames`, the chain the approvals door names people with: one
read for every person of every hop, and none when nothing was substituted.
A person the chain answers no name for is named by the stored id, as every
reader of that chain renders an unresolved address.

The `ooo_substitute` audit comment keeps the ids by design: it is the
request's audit trail of identities, served as written.

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

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

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 9ffe8af094a4c230ec1354521f2efea89eba1f7a → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 9ffe8af094a4c230ec1354521f2efea89eba1f7a

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · domain:services seat 2 · session_018VqJPGA1NDNSCZsnecsChP · 2026-10-17T04:30Z

Reviewed against GitHub at head de3023f846, from the diff and the dev's report on #24065. Clause-②: no: two notification bodies' wording changes, and no export, type or accept set moves. So this is the seat's own review of record.

Form

  • Draft, base main. The first line is Fixes #24065, then Clause-②: no.
  • 3 files, +95 / −4, with one patch changeset for @objectstack/plugin-approvals.
  • check-governed-merges.mjs --pr 24146: NOT governed.

The change. openNodeRequest's OOO fan-out resolves every hop's two people in one resolveUserNames call before the loop. Both bodies name them through oooPerson, which falls back to the stored id.

  • Read by the seat on origin/main: readUserNames returns before any read for an empty list, and both of its reads are in try/catch. So no substitution means no read, and a failed read cannot fail the opening; it names by id, as before.
  • The fallback is the chain's own: the name, else the email for a name-less account row (the label the approvals door already serves), else the stored id. No resolver was added, and the chain's fallback and its field-level-security coupling are untouched.
  • The audit comment keeps ids, by design. The dev read its readers: listActions, the console timeline, the action list views and export. The seat accepts the reasoning: display names belong in read-time *_name companions, the reassign shape, not frozen into an audit row.

The pin. In approval-notification-body.integration.test.ts, on the real service over real ObjectQL:

  • each OOO body carries the person's name and never the id, and a sweep finds no battery user id in any OOO body;
  • a control approver with no account row is named by its id.
  • Red against the unfixed source: the three offending bodies.
  • Ablations: the bare id reds the name pin; dropping the id fallback reds the fallback pin ("… while undefined is out of office."). Both restores are proven by blob.

The open question (the resolveUserNames docblock's STORED COPIES list): answered B. The call-site comment names the bodies as a stored copy and points at that list. A narrowing author greps the chain's callers. The docblock line rides the next change that touches resolveUserNames. A dispatch for one comment line is not worth it.

Out-of-scope findings

  • The console timeline shows ooo_substitute as a raw kind with raw ids in its comment: Acceptance notes. It is a source reading at the pin, with no reach measured. The fix pattern is the reassign shape.
  • On an A → B → C chain, B is told they cover A: Acceptance notes. This is the existing per-hop design.

Gates and CI

  • Gates: 67 derived, 67 exit 0 after the two prerequisite builds, reconciled with --ran.
  • The package passes 81 files and 1107 tests. Typecheck exits 0.
  • CI at de3023f846: 0 failures at this review; Test Core and four lint/typecheck lanes were still running.

ACCEPT. The seat marks the PR ready and enables auto-merge through the fleet relay once CI is green on this head. It lands through the merge queue.


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/s tests tooling

Projects

None yet

2 participants