docs(approvals): qualify the manager approver as a directory-sync dependency - #17575
Merged
os-sales merged 1 commit intoSep 11, 2026
Merged
Conversation
…ependency
`{ type: 'manager' }` is the canonical first rung of a tiered approval ladder
and the type the docs reach for first, but it resolves `sys_user.manager_id` —
a column with no product write surface. An app that authors it passes
`validate` and `lint`, then opens its first request with an empty slate and
waits forever; with the default `lockRecord: true` the record stays locked.
Nothing on the authoring path said so. Three prose additions, no behaviour
change:
- `content/docs/automation/approvals.mdx` — a `manager` callout beside the
existing `position` vs `org_membership_level` one, carrying the remedy text
verbatim from `@objectstack/lint`'s `MANAGER_ONLY_REMEDY` +
`MANAGER_ONLY_ROUTES` so the two statements cannot drift; a pointer from the
"who approves" sentence that first offers the type; and a clause in the
resolve-by-kind paragraph, which listed `manager` unqualified next to the
kinds it already grades.
- `content/docs/capabilities/approvals.mdx` — the capability list offered "the
submitter's direct manager" as a peer of styles that are configured
in-product. Qualified in place.
The routes are stated as measured, not as a promise: a seed or other
system-context write is the one route with a writer in-repo; SCIM and directory
sync are named as something the deployment's own provisioning supplies, because
this platform declares the SCIM `manager` attribute without projecting it and
its admin bulk import excludes the column deliberately.
Claude-Session: https://claude.ai/code/session_01ToDPcx9AESFubJkDiFMtKW
Co-authored-by: Claude <noreply@anthropic.com>
os-sales
marked this pull request as ready for review
September 10, 2026 23:57
os-sales
enabled auto-merge
September 10, 2026 23:58
This was referenced Sep 10, 2026
os-sales
deleted the
claude/issue-16678-manager-approver-sync-dependency
branch
September 11, 2026 00:15
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #16678 — option 3 only, which triage pre-ruled as needing no ruling ("这一条无需裁决,谁认领谁做完"). Option 1 (give
sys_user.manager_ida product write surface) stays on that card and is deliberately left open: it either widens ADR-0092's editable set or opens a new writable field on an authorization surface, which is the maintainer's floor, and triage set the gate that whoever would take it reports back on the card first. Option 2 landed separately as #16748. So this PR is half the card and the card stays open for the other half.Docs prose only. No behaviour change, no exported symbol, no accept-set change.
What changed
content/docs/automation/approvals.mdx(+26)managercallout, placed as a sibling of the existingpositionvsorg_membership_levelone in### 3. The approval node. It carries the remedy text verbatim from@objectstack/lint— see the alignment proof below.:31, "The node declares who approves (a named user, a position, the submitter's manager, …)"). That sentence is where an author learnsmanageris on offer; without the pointer a reader who jumps straight from it to the generated reference never meets the callout.managerunqualified right beside the kinds it already grades (org_membership_level"silently resolves to nobody when it's mistaken for a business hierarchy";queue"not implemented").content/docs/capabilities/approvals.mdx(+13, -1) — see the decision below.The card says the populating routes are "SCIM, an import, or writing to the datastore directly", and triage's acceptance criterion 1 repeats it as «只能经 SCIM / import / 直写数据存储填充». Measured on
f721ef0ff, two of those three are not available on this platform:isUserContextWriteisBoolean(userId) && isSystem !== truein both write guards, so a system-context write bypasses the managed-update whitelist by constructionpackages/spec/src/identity/scim.zod.ts:405declares the Enterprisemanagerattribute, but of the non-test files underpackages/plugins/packages/runtimecarryingmanager_id(6 source files,wc -l8 incl. 2 CHANGELOGs) none writes it: one approvals reader, three sharing-graph readers, and two auth files that name it only to record it is not writableSysScimGroup→ 1 file by the same scan, so the scan discriminatesadmin-import-users.ts→manager_id0 hits;SYS_USER_IMPORT_UPDATE_FIELDSis{name, image, locale}plusphone_numberandrole; andsys-user-writable-fields.ts's own header listsmanager_idamong the admin-surface-only columns, so the omission is deliberatephone_number→ 8 hits, same fileThis does not change the card's shape — the conclusion "no product write surface" is unchanged and in fact stronger. It changes which sentence is true, and criterion 3 (align with #16748) already resolves the conflict in favour of the landed, graded wording. Criterion 3 wins over criterion 1's route list, and that is what this PR writes.
The five facts the new prose asserts, re-measured rather than inherited
On
origin/mainf721ef0ff(the card and triage measured5e53d73d):packages/plugins/plugin-auth/src/identity-write-guard.test.ts:186assertsgetManagedUpdateWhitelist('sys_user')equals exactly{name, image, locale};:132asserts/Editable fields: name, image, locale/. Both reproduce at the same line numbers triage recorded.packages/platform-objects/src/identity/sys-user.object.ts:55carries the matching comment. Source of truth:SYS_USER_PROFILE_EDIT_FIELDSinsys-user-writable-fields.ts. ⛔ Neither assertion is touched by this PR — they are the guard on a deliberate narrowing, not boilerplate.manager_id→ 0 files acrossplugin-auth's route/admin implementations; firing controlphone_number→ 2 files / 9 hits in the same set. Positively pinned in the other direction too:sys-user-self-service-route.test.tsPIN 4 assertsmanager_id"must not be self-editable".manager_id→ 0 hits inpackages/consoleand 0 inpackages/platform-objects/src/apps(excluding translations); firing controllocale→ 3 hits in the same app set. Mechanically it could not be set anyway: the field declaresreadonly: true, andstripReadonlyFieldsdrops areadonlycolumn from any non-system write on both the create and update paths.manager_idunderplatform-objects/metadata-protocol— the four translation bundles (en,es-ES,ja-JP,zh-CN) plus their source-hash files,metadata-protocol/src/seed-loader.ts,sys-user.object.ts:825, and 2 CHANGELOGs. Firing controlprimary_business_unit_id→ 9; nonsense control → 0.⛔ No "future support" wording anywhere in the diff (criterion 2): every sentence is present tense about this tree.
Decision on
content/docs/capabilities/approvals.mdx:11— falsified as written, fixed hereThe line lists, among eight peer "resolution styles", "the submitter's direct manager". It names neither
manager_idnorApproverType, so ⛔ no identifier-based checker can ever reach it — the emitter-blind species.Reason it is falsified: the approver type does resolve at runtime (
lookupManager), so the line is not false about the implementation. What it is false about is availability, and that is the claim a capability list makes: every other style on it gets its data from inside the product — you staff a position, you add people to a team, you name a field on the record — while this one is gated on a column the product cannot write. A buyer reading it concludes they can route to the submitter's manager, and on a stock install with no provisioning they cannot. An unqualified peer in a capability list asserts peer availability, and that assertion does not hold.Fix: the list item is qualified in place (the capability is real once provisioned, so it is not removed) and a callout immediately under the list states the dependency. The callout does not restate the remedy — it links to the one page that carries it verbatim. That is deliberate: a third copy of the remedy is exactly the drift criterion 3 forbids, so the repo holds exactly two statements of it and they are byte-identical.
Also read and deliberately left:
:24, "Multi-level ladders (manager → director → VP) … under 100k ends at manager". That uses "manager" as an org-role noun in an illustrative ladder, not as a claim aboutApproverType; nothing there is falsified.Remedy wording — adopted from #16748, not established, and byte-identity is proven mechanically
#16748 is closed completed (merged 2026-09-09, PR #17034), so its rule text already exists and this PR is the second of the two — the wording was not mine to set. The landed strings are
MANAGER_ONLY_REMEDYandMANAGER_ONLY_ROUTESinpackages/lint/src/validate-approval-approvers.ts, which the rule emits as itshint.Proof, not assertion: extracting the docs paragraph, stripping markdown code delimiters and collapsing whitespace yields 667 characters, and
MANAGER_ONLY_REMEDY + ' ' + MANAGER_ONLY_ROUTESis 667 characters — identical, compared string-to-string.The only transformations are markdown delimiters and line wrapping, both forced and both rendering-neutral:
{name, image, locale}must be fenced in MDX, and the exact failure mode was measured rather than assumed.@mdx-js/mdx@3.1.1(the docs app's own compiler) and reading the emitted JS:<_components.code>{"{name, image, locale}"}</_components.code>— the field set is literal text and the reader sees it;{(name, image, locale)}— a JavaScript sequence expression over three free identifiers, which compiles clean and then fails at render/evaluation instead of showing the field set.⇒ the hazard is real and the fencing is forced, but it is a render-time hazard, not a compile-time one, and I have not measured the Build Docs job itself. The page's existing convention already fences every field set this way.
'manager'in the TypeScript string and`manager`in MDX; the rendered word is the same.sys_user.manager_id) are code-formatted, which is this page's existing style.⛔
packages/lint/**is not touched, and #16748 was not edited — its wording was read.Red lines held
packages/specuntouched.ApproverTypeis declared inpackages/spec/src/automation/approval.zod.ts; no zod,.describe()or JSDoc edit.content/docs/references/automation/approval.mdxuntouched. Its banner was verified first-hand and reads⚠️ AUTO-GENERATED — DO NOT EDIT. Run build-docs.ts to regenerate., namingpackages/spec/src/automation/approval.zod.tsas its source — out of bounds twice over.content/docs/releases/**untouched. ⛔ No test skipped, disabled or quarantined. ⛔ No empty commit.content/docs/; firing controlpackages/→ 31 of the 37, so the filter discriminates. No intersection.content/docs/paths, so the filter fires.Clause-② —
no, and forced rather than chosenThe seat declared
Clause-②: noand the mechanical floor agrees in the same direction: no new exported symbol, no new key on a published payload, no accept-set change. Ayeswould require a package gradedminor+ in the changeset, and a docs-only change that moves nothing published cannot honestly carry one. ⇒needs:contract-reviewstays absent on both this PR and the card. Nothing in the work changed that reading.Changeset —
skip-changeset, measured rather than assumedAGENTS.md: that label "is for a diff that publishes nothing from any released package." Measured:
files[](so the scan reads the field on every one — the firing control), and 0 of 70 namecontentordocsin it.content/docs/is a repo-root directory outside every package directory, andfiles[]is package-relative, so no package can ship it.apps/docs(@objectstack/docs), isprivate: trueand is never published.⇒ nothing published moves. The 14 gate families that "apply once this card's changeset exists" therefore never materialise.
Verification
Gates derived from the actual diff with
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack(stderr confirms the repo assertion holds against this checkout'sorigin, and the change set as 2 paths vs merge basef721ef0ff, three-dot). 40 commands derived, all 40 run, exit code captured before any pipe in every case. Reconciliation with--ran, verbatim verdict line:Five of the 40 first came back non-zero and every one was a prerequisite refusal, not a finding — three
exit 3(@objectstack/lint check:doc-formula-expressions,check:doc-security-posture,check:docs-transcript-drift) and, importantly, twoexit 1(@objectstack/spec check:docs,check:skill-examples) whose printed verdict says a result now "would be computed against a build that no longer matches src". ⭐ Those two are the instructive ones:exit 1there is NOT a finding, and reading the exit code instead of the printed line would have put a false red in this PR. All five were satisfied and re-run to a real 0:pnpm exec turbo run build --filter=@objectstack/formula --filter=@objectstack/lint --filter=@objectstack/spec --concurrency=2→os-verify-lock: VERDICT command-exit 0 · held the lock 159s (2m39s) · waited 0spnpm exec turbo run build --filter=@objectstack/client-react --filter=@objectstack/client --concurrency=2→os-verify-lock: VERDICT command-exit 0 · held the lock 173s (2m53s) · waited 0scheck:skill-examples→✅ 258 prose examples type-check across 3 surface(s)packages/spec/json-schemawas generated by the spec build (not in a merge state — noMERGE_HEAD), andgit status --porcelainis empty after every build:authorable-surface.base.jsondid not move.pnpm lintis disjoint from this diff — a measurement, not a skip. Three readings, as owed:filesglob ineslint.config.mjsis extension-restricted to{ts,tsx,mts,cts,js,jsx,mjs,cjs}..mdxappears in nofilesglob in the file.--format json:pnpm exec eslint --format jsonover both changed paths returns 2 entries,errorCount0, each carrying eslint's own verdict "File ignored because no matching configuration was supplied." ⇒ eslint examined 0 of the 2 files.eslint.config.mjs:327states the repo "never enables type-aware linting (noparserOptions.project, no typed@typescript-eslintrules) for ANY file", and every block'sparserOptionsis{ ecmaVersion: 'latest', sourceType: 'module' }. With no cross-file type program, a diff cannot move any untouched file's verdict — and this diff is not in eslint's population at all.docs-drift, emitter-blind hand search run here rather than delegated — the card's whole point, since the capability page shares no token with the column. Of 62 pages under
content/docs/**mentioning "manager", 29 also mention "approv"; reading every non-releases/one:automation/flows.mdx:50authors{ type: 'position', value: 'manager' }— a position named "manager", a different mechanism, not falsified;automation/workflows.mdx:112andautomation/approvals.mdx:120author{ type: 'field', value: 'owner_manager_id' }— afieldapprover reading a column on the record, notsys_user.manager_id, not falsified;plugins/packages.mdx:372listsmanagerinplugin-approvals' feature list — accurate about the plugin, which does resolve the type;protocol/objectql/schema.mdx:535is a formula over an arbitrary object's ownmanager_id, unrelated;permissions/system-context.mdx:242is narrative prose, not a capability claim;references/automation/approval.mdxis the generated page — out of bounds.⇒ no third page states the manager capability as an unqualified promise. The two fixed here are the complete set.
Both edited pages compile.
@mdx-js/mdx@3.1.1compile()over each file → OK, OK (exit 0). Firing control, run on a copy in scratchpad so the tree was never mutated: the unfenced spelling changes the emitted JS from literal text to a free-identifier expression, as shown above — so the instrument discriminates between the two spellings.⛔ No ablation and no new pin: this is a prose change, and inventing a test to have something to ablate was declined. The MDX compile above is a one-off reading, not a committed test; nothing was left behind.
Acceptance notes
content/docs/capabilities/approvals.mdxsells "a shared queue (first responder claims it)" as one of eight shipped approver styles — the runtime refusesqueueand the slot resolves to nobody #17573: the same page's:11also sells "a shared queue (first responder claims it)" as one of "eight resolution styles", and the runtime refuses it —plugin-approvals/src/approval-service.ts:1716logs "approver type 'queue' is not implemented — the slot resolves to nobody" (firing control'position'→ 4 hits, same file), and the spec gradesqueue: { source: 'unsupported' }. Same harm shape as this card, reached by copying a published sentence. The in-place-fix exemption was checked and declined on one limb — not mechanical: the honest repair moves the headline count in both the## Who approvesheading and the frontmatterdescription, which is an editorial decision about a buyer-facing page's framing. ⭐ Consequence for this PR: the callout added here is worded so it asserts nothing about the other styles or their number — an earlier draft read "Seven of the eight styles above are configured inside the product", which would have been a false sentence of my own.packages/specred line: the generated reference page'sApproverTypelist still offersmanagerwith no qualification, and closing that needs a.describe()edit inpackages/spec/src/automation/approval.zod.ts, which is thedomain:specseat's. The most literal reading of triage's criterion 1 ("ApproverType的文档处") points at exactly that file. This PR satisfies the criterion where an author actually reads — the hand-written authoring page — which is the same conclusion the dispatch's own reading reached.automation/flows.mdx:50authors a position literally namedmanager({ type: 'position', value: 'manager' }) next to a node labelled "Manager Approval". It is correct, and it reads as if it were themanagerapprover type. Successor: none — no PR or person is near that file, and it is a readability observation, not a defect.Authored by the
domain:servicesos-dev agent via Claude Code, sessionsession_01ToDPcx9AESFubJkDiFMtKW, against card #16678.Generated by Claude Code