Skip to content

feat(platform-objects): declare the sys_user set_user_manager row action (#19249) - #19316

Merged
huangyiirene merged 3 commits into
mainfrom
claude/issue-19249-sys-user-set-manager-action
Sep 20, 2026
Merged

huangyiirene merged 3 commits into
mainfrom
claude/issue-19249-sys-user-set-manager-action

Conversation

@huangyiirene

@huangyiirene huangyiirene commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

Part of #19249

⛔ Part of, ⛔ not Fixes — changed by the dispatching seat after review, ⛔ not by the author.
The card's scope item 1 names two behaviours: re-point a manager, and managerId: null clears. This PR delivers the first. The second is not declarable until one reading nobody in this container can take — whether objectui's param dialog submits an explicit null for an untouched optional lookup — because the endpoint refuses an ABSENT managerId and treats only an explicit null as a clear (packages/plugins/plugin-auth/src/admin-set-user-manager.test.ts: :162 pins the null-clear, :175 pins 「an ABSENT managerId is refused, never read as a clear」). ⇒ merging this must not close the card.

Clause-②: no

What this declares

sys_user.manager_id drives the approvals { type: 'manager' } rung and the ADR-0057 own_and_reports read scope, and POST /api/v1/auth/admin/set-user-manager (#16678 Phase 3) has been its only product write surface since it landed — with nothing in the Console reaching it. This declares that affordance and nothing else: one set_user_manager row action on sys_user, offered on list_item and record_header, collecting the manager through an inline sys_user lookup and POSTing { userId, managerId } to the admin endpoint.

Origin ruling (objectstack#16678 Phase 2, decision batch #127 item 1, maintainer 2026-09-13), verbatim:

同意 经理 = 管理员在用户上显式设置的 manager_id;部门负责人 = 单元上的 manager_user_id,两者独立。

So sys_business_unit.manager_user_id (Business Unit Head) is untouched: not read, not written, not derived from or for. The read side is untouched too — manager_id keeps readonly: true and renders in the existing group: 'Organization' exactly as before (re-read on the branch base; ADR-0092 D4).

⛔ It does not write manager_id through the generic data API. sys_user is managedBy: 'better-auth' and the ADR-0092 D2 managed-update whitelist is {name, image, locale}, so that write is refused by the identity write guard — correctly — and the failure would read as a Console bug. The endpoint reaches the column by system context instead, which is why no Tier-1 list moves.

⛔ It declares no second copy of the server's refusals. Self-assignment, cycle, depth, cross-organization and directory-owned identity are all enforced at the write, in one derivation (applyUserManagerLink, which the bulk importer already routes onto rather than re-deriving), and surface from there.

Three readings that changed the shape

1. The visible predicate — the card's count holds; copying the predicate whole would not

The card calls record.source != "idp_provisioned" "the shape the three existing self-service identity actions already use". Measured on the branch base: there are exactly three (change_my_password, change_my_email, delete_my_account), and all three spell that term byte-identically. But all three also AND it with has(record.id) && record.id == ctx.user.id — they are self-service actions, offered to the row owner. This is an admin action on someone else's row, so only the directory-sync term is carried:

visible: 'has(record.source) && record.source != "idp_provisioned"'

Copying the predicate whole would have hidden the button from every admin — silently and fail-closed, the #8990 shape. A per-site verdict pins the counter-direction (offered to an admin on someone else's env_native row) beside the directory-owned verdict, because a guard that is accidentally always-false is user-visibly identical to the bug.

2. No requiresFeature: 'admin' — the one key where this departs from its precedent, deliberately

unlock_user and set_user_password carry it, and the block header states why: those actions hit endpoints "that are only wired when auth.plugins.admin is enabled", so the gate keeps the UI from rendering buttons that 404. This route is not one of them. It is an ObjectStack mount registered unconditionally beside unlock-user in auth-plugin.ts, authorized by the ADR-0068 platform-admin gate (judgePlatformAdmin), never by the better-auth admin plugin. Gating it on features.admin == true would hide a working affordance on every host that never opted into that plugin — precisely the population #16678 measured as having no write surface for this column at all.

There is a second-order consequence worth stating, because it also decides the file surface: PUBLIC_AUTH_FEATURES.admin.gatedInputs in packages/spec enumerates every action gated on that flag, and feature-gate-guard.test.ts reds in its reverse direction when an action carries a features.* term that is not booked there. Declaring the gate would therefore have required a packages/spec edit; not declaring it requires none. The absence is pinned together with that consequence, so a later flip cannot happen without reading it.

3. The spec fork did not fire — and the one half of the suggested route that would have fired it

Everything here uses existing action keys: type, target, recordIdParam, visible, description, successMessage, refreshAfter, and a params[] entry of type: 'lookup' with reference. No new or widened packages/spec key, no spec file touched, so Clause-②: no holds.

The half that is not declarable is "a user lookup filtered to the same organization". ActionParamSchema is strict and declares no filter key at all; the only structured picker filter in the schema is FieldSchema.lookupFilters, whose entries are literal { field, operator, value } triples with no context token — and sys_user carries no organization_id column to filter on, being a global identity table (sys_member rows are the only tenancy fact either identity has, which is exactly why the endpoint's cross-organization screen reads sys_member). So the org-scoping half has no existing-key spelling, and what does exist is the server's named cross_organization refusal. A client-side approximation of it would have been the second copy this card forbids, so the picker is left unscoped and the refusal surfaces.

Verification

Gate families derived from the actual diff with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, every command run with its exit code captured before any pipe, reconciled with --ran:

dispatch-gates --ran: 58 derived famil(ies) accounted for — 58 run,
0 NOT-MEASURED (a DERIVED zero — all 58 recorded an exit code and none of them is 3).
check result
58 derived gate families all exit 0
pnpm --filter @objectstack/platform-objects test 41 files / 584 tests passed
pnpm --filter @objectstack/platform-objects typecheck OK (incl. check:test-typecheck)
pnpm build 73/73 tasks
pnpm check:i18n OK — 9 packages, all bundles in sync
pnpm check:i18n-coverage OK — 13 configs, 621 baselined untranslated strings, none new
pnpm lint (eslint . --no-inline-config, whole repo) exit 0 at b0131a89f

pnpm check:dual-build-cjs-loads first answered exit 3 — PREREQUISITE NOT MET (no dist/ for 12 packages). That is not a pass, so the prerequisite was cleared with a full pnpm build and the gate re-run: exit 0. Control-character self-scan over all seven changed files: no match.

The i18n bundles were regenerated with node scripts/check-i18n-bundles.mjs --write, and the three translated locales were then hand-translated rather than left as the extractor's English fill — the #7309 trap: an English value in a non-English bundle is perfectly "in sync" to check:i18n and invisible to every gate. The generated source-hash tables drop their entries for a re-translated leaf by themselves, which is why they carry no diff here.

Acceptance notes

  • Clearing the link has no affordance in this action, and that is a declared narrowing rather than an oversight. The endpoint's clear path requires managerId to be present and null — "managerId is required — send null to clear the link, never omit the key" — and refuses both an absent key and an empty string. Whether the Console's param dialog submits an explicit null for an untouched optional lookup is objectui behaviour, and objectui is not checked out in this container, so it could not be measured here. Rather than half-declare it, the param is required: true: the dialog collects a value before anything is POSTed, so no submit path can produce that 400 about a key the user never saw. Re-pointing a manager works; unsetting one still needs either a measured defaultValue: null path or a companion action carrying bodyExtra: { managerId: null } — one existing-key line either way, on a measurement this container cannot take. Noted, not filed; successor: the next author of a sys_user action, whom this note and the pin in sys-user-set-manager-action.test.ts both reach.
  • The file surface ran wider than the dispatch's packages/platform-objects/src/identity/ line, mechanically and in one direction only. A new action label, description, success message and param label are authorable i18n keys, so pnpm check:i18n reds until src/apps/translations/*.objects.generated.ts is regenerated. Four bundle files outside identity/, all generated-then-translated, no hand-written structure. Flagged rather than silently widened.
  • Noted, not filed: unlock_user carries requiresFeature: 'admin' while its own route is mounted unconditionally on the raw app, so on a host without the better-auth admin plugin the Unlock Account button is hidden although the endpoint answers. Same class as the reading in §2 above, on an action this PR does not touch. Successor: whoever next revisits the #2874 feature-gate roster.
  • Noted, not filed: sys_user.manager_id's own field help string is untranslated English in all three translated bundles (pre-existing, inside the 621 baselined strings this PR leaves flat).

Generated by Claude Code

…19249)

Declares the `set_user_manager` row action on `sys_user`, posting
`POST /api/v1/auth/admin/set-user-manager` with `{ userId, managerId }`.
The endpoint (#16678 Phase 3) already exists and is ledgered; this is the
Console affordance that reaches it.

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

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/platform-objects, touching 8 documentable anchor(s).

28 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 e3b3cdd2df3bda349ef7a41b0de39c8de4fddc87.

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

What this run could not see
  • 2 anchor(s) matched too much of the corpus to be a work list: sys_user (symbol, 36 pages), sys_user (literal, 36 pages)
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 3 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 e3b3cdd2df3bda349ef7a41b0de39c8de4fddc87 → packageMentionDocs.

Which tree this was computed on

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

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

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

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 20, 2026
@huangyiirene
huangyiirene marked this pull request as ready for review September 20, 2026 11:26
@huangyiirene
huangyiirene added this pull request to the merge queue Sep 20, 2026
Merged via the queue into main with commit 74fb2f7 Sep 20, 2026
43 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-19249-sys-user-set-manager-action branch September 20, 2026 12:02
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