Skip to content

docs(approvals): qualify the manager approver as a directory-sync dependency - #17575

Merged
os-sales merged 1 commit into
mainfrom
claude/issue-16678-manager-approver-sync-dependency
Sep 11, 2026
Merged

docs(approvals): qualify the manager approver as a directory-sync dependency#17575
os-sales merged 1 commit into
mainfrom
claude/issue-16678-manager-approver-sync-dependency

Conversation

@claude

@claude claude Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Part of #16678option 3 only, which triage pre-ruled as needing no ruling ("这一条无需裁决,谁认领谁做完"). Option 1 (give sys_user.manager_id a 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)

  • A manager callout, placed as a sibling of the existing position vs org_membership_level one in ### 3. The approval node. It carries the remedy text verbatim from @objectstack/lint — see the alignment proof below.
  • A pointer on the sentence that first offers the type (:31, "The node declares who approves (a named user, a position, the submitter's manager, …)"). That sentence is where an author learns manager is on offer; without the pointer a reader who jumps straight from it to the generated reference never meets the callout.
  • A clause in the resolve-by-kind paragraph, which listed manager unqualified 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.

⚠️ One premise in the card's own text does not reproduce, and the correction is already landed wording

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:

route reading control
seed / any system-context write availableisUserContextWrite is Boolean(userId) && isSystem !== true in both write guards, so a system-context write bypasses the managed-update whitelist by construction
SCIM not herepackages/spec/src/identity/scim.zod.ts:405 declares the Enterprise manager attribute, but of the non-test files under packages/plugins / packages/runtime carrying manager_id (6 source files, wc -l 8 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 writable SysScimGroup1 file by the same scan, so the scan discriminates
admin bulk import not hereadmin-import-users.tsmanager_id 0 hits; SYS_USER_IMPORT_UPDATE_FIELDS is {name, image, locale} plus phone_number and role; and sys-user-writable-fields.ts's own header lists manager_id among the admin-surface-only columns, so the omission is deliberate phone_number8 hits, same file

This 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/main f721ef0ff (the card and triage measured 5e53d73d):

  1. The data API refuses the column, and it is pinned. packages/plugins/plugin-auth/src/identity-write-guard.test.ts:186 asserts getManagedUpdateWhitelist('sys_user') equals exactly {name, image, locale}; :132 asserts /Editable fields: name, image, locale/. Both reproduce at the same line numbers triage recorded. packages/platform-objects/src/identity/sys-user.object.ts:55 carries the matching comment. Source of truth: SYS_USER_PROFILE_EDIT_FIELDS in sys-user-writable-fields.ts. ⛔ Neither assertion is touched by this PR — they are the guard on a deliberate narrowing, not boilerplate.
  2. Nothing under the auth routes accepts a manager. manager_id0 files across plugin-auth's route/admin implementations; firing control phone_number2 files / 9 hits in the same set. Positively pinned in the other direction too: sys-user-self-service-route.test.ts PIN 4 asserts manager_id "must not be self-editable".
  3. No UI surface offers it. manager_id0 hits in packages/console and 0 in packages/platform-objects/src/apps (excluding translations); firing control locale3 hits in the same app set. Mechanically it could not be set anyway: the field declares readonly: true, and stripReadonlyFields drops a readonly column from any non-system write on both the create and update paths.
  4. It is a declared column, not a phantom field. 11 files carry manager_id under platform-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 control primary_business_unit_id9; nonsense control → 0.
  5. The populating routes — the table above. Stated as a graded measurement, never as a promise.

⛔ 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:11falsified as written, fixed here

The line lists, among eight peer "resolution styles", "the submitter's direct manager". It names neither manager_id nor ApproverType, 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 about ApproverType; 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_REMEDY and MANAGER_ONLY_ROUTES in packages/lint/src/validate-approval-approvers.ts, which the rule emits as its hint.

Proof, not assertion: extracting the docs paragraph, stripping markdown code delimiters and collapsing whitespace yields 667 characters, and MANAGER_ONLY_REMEDY + ' ' + MANAGER_ONLY_ROUTES is 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. ⚠️ My first statement of this was wrong: unfenced braces do not break the MDX compile. Compiling both spellings through @mdx-js/mdx@3.1.1 (the docs app's own compiler) and reading the emitted JS:
    • fenced (shipped) → <_components.code>{"{name, image, locale}"}</_components.code> — the field set is literal text and the reader sees it;
    • unfenced (control) → {(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.
  • The SCIM attribute is spelled 'manager' in the TypeScript string and `manager` in MDX; the rendered word is the same.
  • Identifiers (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/spec untouched. ApproverType is declared in packages/spec/src/automation/approval.zod.ts; no zod, .describe() or JSDoc edit.
  • content/docs/references/automation/approval.mdx untouched. Its banner was verified first-hand and reads ⚠️ AUTO-GENERATED — DO NOT EDIT. Run build-docs.ts to regenerate., naming packages/spec/src/automation/approval.zod.ts as its source — out of bounds twice over.
  • content/docs/releases/** untouched. ⛔ No test skipped, disabled or quarantined. ⛔ No empty commit.
  • Fence re-verified, not inherited: PR feat(auth)!: adopt better-auth's account-issuer rollback — drop sys_account.issuer, retire the backfill, lift the family to 1.7.3 #17454's file list is 37 paths, of which 0 are under content/docs/; firing control packages/31 of the 37, so the filter discriminates. No intersection.
  • Collision sweep: all 22 open PRs' file lists fetched, the 295-file one paged to exhaustion (100 + 100 + 95 + 0) — 0 hits on either file this PR touches. Control: every one of the 22 returned a non-empty list, and page 2 of the large one does contain 3 content/docs/ paths, so the filter fires.

Clause-② — no, and forced rather than chosen

The seat declared Clause-②: no and the mechanical floor agrees in the same direction: no new exported symbol, no new key on a published payload, no accept-set change. A yes would require a package graded minor+ in the changeset, and a docs-only change that moves nothing published cannot honestly carry one. ⇒ needs:contract-review stays absent on both this PR and the card. Nothing in the work changed that reading.

Changeset — skip-changeset, measured rather than assumed

AGENTS.md: that label "is for a diff that publishes nothing from any released package." Measured:

  • 70 non-private packages, 70 of 70 declare a files[] (so the scan reads the field on every one — the firing control), and 0 of 70 name content or docs in it.
  • Structurally stronger: content/docs/ is a repo-root directory outside every package directory, and files[] is package-relative, so no package can ship it.
  • Its only consumer, apps/docs (@objectstack/docs), is private: true and 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's origin, and the change set as 2 paths vs merge base f721ef0ff, three-dot). 40 commands derived, all 40 run, exit code captured before any pipe in every case. Reconciliation with --ran, verbatim verdict line:

✓ dispatch-gates --ran: 40 derived famil(ies) accounted for — 40 run, 0 NOT-MEASURED (a DERIVED zero — all 40 recorded an exit code and none of them is 3).

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, two exit 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 1 there 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=2os-verify-lock: VERDICT command-exit 0 · held the lock 159s (2m39s) · waited 0s
  • pnpm exec turbo run build --filter=@objectstack/client-react --filter=@objectstack/client --concurrency=2os-verify-lock: VERDICT command-exit 0 · held the lock 173s (2m53s) · waited 0s
  • then check:skill-examples✅ 258 prose examples type-check across 3 surface(s)

packages/spec/json-schema was generated by the spec build (not in a merge state — no MERGE_HEAD), and git status --porcelain is empty after every build: authorable-surface.base.json did not move.

pnpm lint is disjoint from this diff — a measurement, not a skip. Three readings, as owed:

  1. Population, read from eslint's own config: every block's files glob in eslint.config.mjs is extension-restricted to {ts,tsx,mts,cts,js,jsx,mjs,cjs}. .mdx appears in no files glob in the file.
  2. Count, read from --format json: pnpm exec eslint --format json over both changed paths returns 2 entries, errorCount 0, each carrying eslint's own verdict "File ignored because no matching configuration was supplied." ⇒ eslint examined 0 of the 2 files.
  3. Invariance for untouched files: eslint.config.mjs:327 states the repo "never enables type-aware linting (no parserOptions.project, no typed @typescript-eslint rules) for ANY file", and every block's parserOptions is { 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:

  • the two pages in this PR — fixed;
  • automation/flows.mdx:50 authors { type: 'position', value: 'manager' } — a position named "manager", a different mechanism, not falsified;
  • automation/workflows.mdx:112 and automation/approvals.mdx:120 author { type: 'field', value: 'owner_manager_id' } — a field approver reading a column on the record, not sys_user.manager_id, not falsified;
  • plugins/packages.mdx:372 lists manager in plugin-approvals' feature list — accurate about the plugin, which does resolve the type;
  • protocol/objectql/schema.mdx:535 is a formula over an arbitrary object's own manager_id, unrelated;
  • permissions/system-context.mdx:242 is narrative prose, not a capability claim;
  • references/automation/approval.mdx is 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.1 compile() 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

  • Filed as content/docs/capabilities/approvals.mdx sells "a shared queue (first responder claims it)" as one of eight shipped approver styles — the runtime refuses queue and the slot resolves to nobody #17573: the same page's :11 also 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:1716 logs "approver type 'queue' is not implemented — the slot resolves to nobody" (firing control 'position' → 4 hits, same file), and the spec grades queue: { 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 approves heading and the frontmatter description, 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.
  • Reported to the dispatching seat, ⛔ not filed — it sits behind the packages/spec red line: the generated reference page's ApproverType list still offers manager with no qualification, and closing that needs a .describe() edit in packages/spec/src/automation/approval.zod.ts, which is the domain:spec seat'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.
  • noted, not filed: automation/flows.mdx:50 authors a position literally named manager ({ type: 'position', value: 'manager' }) next to a node labelled "Manager Approval". It is correct, and it reads as if it were the manager approver type. Successor: none — no PR or person is near that file, and it is a readability observation, not a defect.

Authored by the domain:services os-dev agent via Claude Code, session session_01ToDPcx9AESFubJkDiFMtKW, against card #16678.


Generated by Claude Code

…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>
@claude claude Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 10, 2026
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Sep 10, 2026
@os-sales
os-sales marked this pull request as ready for review September 10, 2026 23:57
@os-sales
os-sales enabled auto-merge September 10, 2026 23:58
@os-sales
os-sales added this pull request to the merge queue Sep 10, 2026
Merged via the queue into main with commit ef18030 Sep 11, 2026
49 checks passed
@os-sales
os-sales deleted the claude/issue-16678-manager-approver-sync-dependency branch September 11, 2026 00:15
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 skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants