Skip to content

docs(spec): ApproverType.describe() qualifies manager instead of offering it as a bare allowed value - #17640

Merged
os-bill merged 2 commits into
mainfrom
claude/issue-17579-approver-type-manager-describe
Sep 11, 2026
Merged

os-bill merged 2 commits into
mainfrom
claude/issue-17579-approver-type-manager-describe

Conversation

@os-bill

@os-bill os-bill commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Part of #17579

  • Clause-②: no — this PR puts no new key on any published payload. ApproverType's accept set is byte-identical (no member added, removed or renamed), check:api-surface is green on the rebuilt dist/*.d.ts, and check:authorable-surface left the checked-in artifacts untouched. The only new bytes are prose.

What this changes

ApproverType carried no .describe() at all, so the generated reference page rendered ## ApproverType with nothing but an ### Allowed Values list. manager — the one rung an author cannot operate on a stock install — read exactly like the nine members that work.

  • packages/spec/src/automation/approval.zod.ts — the enum gains a .describe() that qualifies manager and points at the remedy instead of restating it.
  • content/docs/references/automation/approval.mdxregenerated, never hand-edited (see the generator evidence below).
  • packages/lint/src/validate-approval-approvers.ts — one paragraph added to the DEPENDENCY docblock above MANAGER_ONLY_REMEDY, naming the new describe among the lines that go stale if manager_id ever gains a write surface. The triage comment called this out as the part a round is most likely to skip; it is a comment only.

The describe, verbatim:

Approval step approver type. manager is a directory-sync dependency rather than something an author configures here: it resolves the submitter's sys_user.manager_id at runtime, and that column has no product write surface, so until an operator populates it from outside the product a manager step resolves to nobody and the request waits. os lint reports that at authoring time as approval-approvers-may-resolve-empty and carries the graded population routes and the full remedy; the Approvals guide states the same remedy in prose.

Route (iii), the triage default: it points, it does not restate. MANAGER_ONLY_REMEDY / MANAGER_ONLY_ROUTES stay the single authoritative copy of the 667-character remedy. No third copy was written, so there is nothing new to keep in step — a pointer cannot drift into disagreement with what it points at. ⛔ No export was added to packages/spec (route (ii) was fenced).

Prerequisite readings — taken by state on origin/main @ ea2940d1c4, not inherited from the card

1. The manager describe. ⚠️ Correction to the card: ApproverType had no .describe() to read unqualified — it had none. Its JSDoc (approval.zod.ts:24-30) renders nowhere: renderSchemaSection (packages/spec/scripts/lib/schema-section.ts:328-330) prints mainDef.description, which only .describe() / .meta({description}) fills. Positive control that the mechanism is real: HttpMethod's .describe() (packages/spec/src/shared/http.zod.ts:45) renders at content/docs/references/shared/http.mdx:46. The card's substance stands — the page sold manager unqualified — but the fix is a describe added, not a describe edited.

2. The generated page. content/docs/references/automation/approval.mdx:113 is ## ApproverType, :115 is ### Allowed Values, :117 is the manager bullet — the exact window the card names. Its banner (:6) still reads ⚠️ AUTO-GENERATED — DO NOT EDIT. Run build-docs.ts to regenerate. and the source callout (:9-10) still names packages/spec/src/automation/approval.zod.ts. Both hold.

3. ⭐ sys_user.manager_id has no product write surface — re-measured, with the negative leg lit.

A throwaway probe drove the identity write guard's real beforeUpdate handler in a user context, then was deleted (working tree verified clean afterwards); both legs went through the same handler:

PROBE legA surviving payload = {"id":"u1","name":"Keep"}      # manager_id stripped
PROBE legB surviving payload = {"id":"u1","locale":"zh-CN"}   # control survives intact
PROBE whitelist = ["name","image","locale"]
Test Files  2 passed (2)   Tests  29 passed (29)
  • Leg A (subject). { id, name, manager_id } came back as { id, name }manager_id never reaches the row. { id, manager_id } alone is refused loudly: PERMISSION_DENIED / 403.
  • Leg B (negative leg). locale — a field that IS writable — passed the same probe unchanged. ⇒ the instrument is not a refusal that refuses everything.
  • The landed pins the card cites are still exactly where it says: identity-write-guard.test.ts:132 (/Editable fields: name, image, locale/) and :186 (getManagedUpdateWhitelist('sys_user') equals new Set(['name','image','locale'])). Both green in the same run.

readonly: true on the column, with a discriminating control — readonly occurrences inside each field block of packages/platform-objects/src/identity/sys-user.object.ts:

field readonly occurrences
manager_id 1
primary_business_unit_id 1
locale (control — writable) 0
name (control — writable) 0

No accepting route: across the 59 non-test packages/plugins/plugin-auth/src/*.ts files (enumerated with git ls-tree, not by grepping contents for a filename), manager_id occurs 5 times against a firing control of phone_number at 28 — and all 5 sit in managed-extension-fields.ts and sys-user-writable-fields.ts, both of which name the column only to record that it is not writable (MANAGED_EXTENSION_EDITABLE_FIELDS.sys_user is new Set(['locale'])). Counts are grep -o | wc -l, not grep -c.

The premise holds. premise_still_valid: true.

4. What PR #17575 already fixed — not redone. The manager callout in content/docs/automation/approvals.mdx:65-87 and the qualification in content/docs/capabilities/approvals.mdx:14-19 are both present and untouched by this PR (git diff names neither file).

The page was regenerated, not hand-edited

$ pnpm --filter @objectstack/spec build           # required first (the dist caveat)
$ pnpm --filter @objectstack/spec check:generated
  ✗ check:docs   content/docs/references/**       # 1 of 15 stale — exactly the one predicted
$ pnpm --filter @objectstack/spec gen:docs
  ✅ Generated 222 files
$ git status --porcelain
   M content/docs/references/automation/approval.mdx     # 1 of the 222 changed
$ pnpm --filter @objectstack/spec check:generated
  ✓ All 15 generated artifacts are up to date.

The generator rewrote all 222 reference pages and exactly one moved. Beyond the ## ApproverType section, the describe also reached the previously blank Description cells of the type rows in ApprovalNodeApprover and ApprovalNodeConfig.approvers — measured on the regenerated output, not predicted from the zod registry semantics (which read the other way).

Verification

  • pnpm --filter @objectstack/spec check:generatedall 15 green, including check:api-surface, check:authorable-surface, check:docs.
  • Derived gate familynode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack over this diff: 100 families derived, 98 run green, 0 unrun, 2 NOT MEASURED. Reconciled with --ran carrying each recorded exit code.
    • pnpm check:dual-build-cjs-loads and pnpm check:lean-entry-closure both exit 3 — PREREQUISITE NOT MET (they load built entry points repo-wide and this worktree has no full pnpm build). ⛔ Recorded as NOT MEASURED, not as green; CI builds and runs both.
    • One run was discarded and re-run rather than silently retried: check:skill-examples first exited 1 for want of packages/client-react/dist, a prerequisite refusal, not a red. After pnpm --filter @objectstack/client-react build it is green — ✅ 258 prose examples type-check across 3 surface(s).
    • A second discarded run: pnpm --filter … run test --concurrency=2 forwarded --concurrency into the vitest script and failed on flag parsing. Re-run correctly as pnpm --workspace-concurrency=2 --filter … run test.
  • pnpm --workspace-concurrency=2 --filter @objectstack/spec --filter @objectstack/lint run test — spec 473 files / 13429 tests passed, lint 103 files / 3747 tests passed.
  • pnpm --workspace-concurrency=2 --filter @objectstack/spec --filter @objectstack/lint run typecheck — exit 0, both test-layer debt ledgers held.
  • pnpm lint (eslint . --no-inline-config, the whole repo) — exit 0. No narrowing was needed, so none is claimed.
  • pnpm check:nul-bytes green, plus a direct control-character scan over the four touched files (grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]' → no match).

Heavy runs went through scripts/pm/os-verify-lock.sh; the verdicts above are its VERDICT command-exit lines, with each exit code captured before any pipe.

Changeset

.changeset/17579-approver-type-manager-describe.md@objectstack/spec: patch. The describe ships: @objectstack/spec's files[] carries dist, json-schema and src/**/*.zod.ts, and the new string is measured in all three on the built tree — dist/automation/index.js + .mjs (2 files, against a lit control of an existing describe from the same module, also 2), four json-schema/ documents, and the shipped approval.zod.ts source. Prose only, no surface widening ⇒ patch, not minor.

@objectstack/lint is deliberately not graded. It publishes dist only, and the new docblock sentence is absent from it (0 files) while a runtime string from the same source file is present in 4 and a pre-existing comment from the same docblock is absent in 0 — comments are stripped by construction, so nothing published moves there.

Out of scope


Generated by Claude Code

…fering it bare

`ApproverType` carried no `.describe()` at all, so the generated reference page
`content/docs/references/automation/approval.mdx` rendered `## ApproverType`
with nothing but an `### Allowed Values` list: `manager` — the one rung an
author cannot operate on a stock install, because `sys_user.manager_id` has no
product write surface — read exactly like the nine members that work.

The describe qualifies it and POINTS at the remedy rather than restating it:
`MANAGER_ONLY_REMEDY` / `MANAGER_ONLY_ROUTES` in
`packages/lint/src/validate-approval-approvers.ts` stay the single
authoritative copy, and that file's `DEPENDENCY` docblock now names this new
string among the lines that go stale if the column ever gains a write surface.

No enum member is added, removed or renamed.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH
`pnpm --filter @objectstack/spec gen:docs` rewrote all 222 reference pages;
exactly one changed. The `## ApproverType` section now carries the qualifying
describe above its `### Allowed Values` list, and the `type` rows of
`ApprovalNodeApprover` and `ApprovalNodeConfig.approvers` — previously blank —
carry it too. The page was never hand-edited.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/lint, @objectstack/spec, touching 1 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/lint/src/validate-approval-approvers.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

  • content/docs/automation/approvals.mdx (via ApproverType (symbol, a top-level const object))

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

  • content/docs/releases/v16.mdx (via ApproverType (symbol, a top-level const object))

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
  • 1 changed file(s) yielded no anchor (packages/lint/src/validate-approval-approvers.ts) — pages documenting those are invisible to this run
  • 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.

Coarse fallback — 135 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 76c9fab30ca406b7b1f06b8ca3db286af9f2bf8bpackageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 76c9fab30ca406b7b1f06b8ca3db286af9f2bf8b

⚠️ 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 76c9fab30ca406b7b1f06b8ca3db286af9f2bf8b → 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 tooling labels Sep 11, 2026
@os-bill
os-bill marked this pull request as ready for review September 11, 2026 07:48
@os-bill
os-bill enabled auto-merge September 11, 2026 07:48
@os-bill
os-bill added this pull request to the merge queue Sep 11, 2026
Merged via the queue into main with commit 2eb4724 Sep 11, 2026
36 checks passed
@os-bill
os-bill deleted the claude/issue-17579-approver-type-manager-describe branch September 11, 2026 08:17
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 tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants