Skip to content

docs(client): README approval decisions are objects, and register carries its required name - #16926

Merged
os-project-manager merged 1 commit into
mainfrom
claude/issue-16210-readme-approvals-decision-object
Sep 8, 2026
Merged

docs(client): README approval decisions are objects, and register carries its required name#16926
os-project-manager merged 1 commit into
mainfrom
claude/issue-16210-readme-approvals-decision-object

Conversation

@os-project-manager

@os-project-manager os-project-manager commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Fixes #16210

The published README's namespace tour called approvals.approve / approvals.reject with a bare comment string. Both declare a decision object as the second parameter, so the comment was never going to arrive:

approve: async (requestId: string, decision?: { actorId?: string; comment?: string; attachments?: string[] }): Promise<ApprovalDecisionResult> => {
reject:  async (requestId: string, decision?: { actorId?: string; comment?: string; attachments?: string[] }): Promise<ApprovalDecisionResult> => {

Triage's reason for grading this p2 rather than cosmetic, kept in its own words:

JavaScript 读者没有类型错误,评论被静默丢弃 —— 那个字符串被当作请求体发出去,而路由期望的是决策对象的字段。⇒ 审批被记下,理由消失,没有任何东西报错。

packages/client/package.json declares files: ["dist", "README.md", "CHANGELOG.md"], so this file is the npm front page of @objectstack/client.

Clause-②

Clause-②: no
— Re-declared from the delivered diff, not from the dispatch. The diff is packages/client/README.md
plus one changeset: prose narrowed back onto the signature the package already publishes. No accept
set is relaxed, no published surface gains a member, and no schema, export or type moved in either
direction. The auth.register correction is the same shape — the README now passes the name that
RegisterRequestSchema has always declared required; the contract did not move, the example did.

Located by symbol, never by line

The card cites :4686 / :4699 and triage recorded those rotted to "around :5283". They have rotted again: on origin/main @ ed6579b53b the symbols sit at packages/client/src/index.ts:5293 (approve) and :5306 (reject). Both were found by symbol. The README fence was located by content (the client.approvals.approve( call), not by offset.

The executable acceptance — both directions

Harness: the fence is extracted from packages/client/README.md by content, wrapped in an async function, and type-checked against the package's own built declarations (packages/client/dist/index.d.ts) after pnpm --filter '@objectstack/client...' build (green, 419s under the shared verify lock). Placeholders the prose uses without declaring are typed from the declarations themselves so they stay neutral: requestId/id/userId as string, batchRequest as the exported BatchUpdateRequest, fileData as unknown (the upload parameter is any).

Pre-fix — RED (tsc exit 2). Verbatim:

packages/client/.tscheck-16210/tour.ts(31,30): error TS2345: Argument of type '{ email: string; password: string; }' is not assignable to parameter of type '{ email: string; password: string; name: string; image?: string | undefined; }'.
  Property 'name' is missing in type '{ email: string; password: string; }' but required in type '{ email: string; password: string; name: string; image?: string | undefined; }'.
packages/client/.tscheck-16210/tour.ts(46,45): error TS2559: Type '"LGTM"' has no properties in common with type '{ actorId?: string | undefined; comment?: string | undefined; attachments?: string[] | undefined; }'.
packages/client/.tscheck-16210/tour.ts(47,44): error TS2559: Type '"Incomplete"' has no properties in common with type '{ actorId?: string | undefined; comment?: string | undefined; attachments?: string[] | undefined; }'.

Post-fix — GREEN. Re-generated from the corrected README and re-run unchanged, at final commit aaff32ce65:

FINAL_TSC_EXIT=0
fence: README lines 222..291 (opener/closer), 68 body lines
calls in fence: 35

The harness lived in an untracked scratch directory and was removed; git status is clean and it is not in this diff.

Whole-fence sweep — 35 calls, not 2

Per triage's instruction to widen the reading without widening the diff, every call in the fence went through the same .d.ts check, not just the two the card reports. Three hits, classified:

Hit Call Error Class Disposition
1 approvals.approve(requestId, 'LGTM') TS2559 live method, wrong argument shape fixed here
2 approvals.reject(requestId, 'Incomplete') TS2559 live method, wrong argument shape fixed here
3 auth.register({ email, password }) TS2345 live method, wrong argument shape fixed here

Hit 3 is the one the card did not report, and it is the same defect class, not #16142's: auth.register exists and is called with the wrong argument shape. RegisterRequestSchema (packages/spec/src/api/auth.zod.ts) declares name: z.string() as required and packages/spec/src/api/auth.test.ts pins the rejection of a request without it, so the contract is settled upstream and the README was the copy left behind — the same asymmetry the card describes. Per the dispatch's rule for this class it is fixed in this PR rather than filed.

Zero for the other class, with its control. No call in the fence failed as a missing method (#16142's class, which #16212 already closed), so nothing was filed separately. That zero is a reading rather than a silence because the same harness, on the same text, fired three errors — hits 1-3 above are the control.

All three corrections are aligned to content/docs/api/client-sdk.mdx, which already carries the right spelling for both (:359-360 for the approvals pair, :350 for register). After the fix, all 35 calls in the fence type-check clean.

Docs drift — the bot declined to answer, so this is a hand sweep

The docs-drift bot returned a non-answer, not a pass:

⚠️ 1 changed file(s) yielded no anchor (packages/client/README.md), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

A README yields no symbol/route/SDK anchor, so that zero is definitional rather than measured. It is replaced here by a hand sweep taken on this branch at aaff32ce65 — ⛔ not treated as a pass. The alignment note above answers "is the docs page right?"; this section answers the different question "does any other page carry the same wrong shape?".

Query — a decision call whose second argument opens with a quote, i.e. the exact defect this card fixes:

grep -rnE "approvals\.(approve|reject)\([^,)]+,\s*['\"]"

Result: 0 hits on content/. 0 hits repo-wide (excluding node_modules, dist, .git).

Two controls, so that zero is a reading and not a dead grep:

  1. The corpus is reached. The shape-agnostic form of the same query over content/ returns content/docs/api/client-sdk.mdx:359-360 — the card's own reference spelling, already correct. The query demonstrably walks the docs corpus.
  2. The regex fires. The wrong-shape regex above, run unchanged against the pre-fix README (git show ed6579b53b:packages/client/README.md), matches lines 255-256. So a zero elsewhere means "no wrong shape", not "broken pattern".

Every approvals.approve / .reject site on this tree, read out rather than counted:

  • content/docs/api/client-sdk.mdx:359-360 — already correct object form (this is control leg 1).
  • packages/client/src/client.test.ts:783,798 — correct object form.
  • packages/client/README.md:255-256 — the defect this PR fixes; now correct.
  • Five CHANGELOG files carry client.approvals.approve(requestId, …) with the argument elided, so they assert no shape: packages/client, packages/runtime, packages/spec, packages/services/service-automation, packages/plugins/plugin-approvals.
  • packages/rest/src/rest-route-ledger.ts:429-430 — ledger rows whose client: field holds the string 'approvals.approve'; not calls.
  • docs/qa/platform-checklist/**approvals.approver-resolution-matrix, an unrelated identifier matched on substring only.

And for hit 3's method, every auth.register site:

  • content/docs/permissions/authentication.mdx:213 and :1009 — both multi-line calls, both carry name: 'John Doe'. Correct.
  • content/docs/api/client-sdk.mdx:350 — correct.
  • packages/client/src/client.test.ts:822 — carries name: 'Test User'. Correct.
  • packages/plugins/plugin-auth/src/auth-route-ledger.ts:169 — ledger row, not a call.
  • packages/cli/src/commands/register.ts:128 — not a docs site; recorded under 验收备注 below.

content/ carries no second instance of either defect.

One difference from the PM's reading, reported rather than reconciled silently. The PM read four CHANGELOGs at origin/main@9a89a0040d; this tree at aaff32ce65 has five. The two readings agree on the substance — the elided (requestId, …) form asserts no argument shape either way — and the discrepancy is a file count in a corpus that grows per release, taken on two different commits. The five is the number measured on the tree this PR delivers.

File surface

  • packages/client/README.md — the only content change.
  • .changeset/client-readme-approvals-decision-object.md@objectstack/client patch. The README is a published file of this package, so the published artifact moves even though no code does; the changeset body says so.

packages/client/src/index.ts was READ ONLY — opened only to locate the approve / reject symbols and the register signature. It is held by open PR #16761 and is not in this diff. No source change was needed, so the stop-and-report condition for a widening diff never triggered.

No governed surface is in the diff: no docs/adr/**, no .claude/**, no skills/**, no AGENTS.md, no CLAUDE.md. No content/docs/releases/ page is touched — this PR's only input to the release notes is its changeset.

The missing gate is not touched: that this whole fence sits in check:published-readme-exports's NOT read bucket is #16209's subject, already filed and measured.

Gates

Derived with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack from the real change set (2 paths, three-dot against the merge base), re-derived identically after git fetch origin main.

47 derived families, 47 run, 0 NOT-MEASURED, 0 UNRUN — reconciled with --ran:

dispatch-gates --ran: 47 derived famil(ies) accounted for — 47 run, 0 NOT-MEASURED.

Two of the 47 first came back PREREQUISITE NOT MET (check:skill-examples, check:dual-build-cjs-loads) because they read built output the worktree did not yet have. That is not a red and not a pass — nothing was measured. A full pnpm build --concurrency=2 (73/73 tasks successful, under the shared verify lock) was run and both were re-run properly:

GATE30_EXIT=0   pnpm --filter @objectstack/spec run check:skill-examples
  258 prose examples type-check across 3 surface(s)
GATE36_EXIT=0   pnpm check:dual-build-cjs-loads
  104 published require entry point(s) across 67 package(s) load

Check Changeset's subject is satisfied: this PR adds exactly one changeset, which is what that job counts. check-changeset-no-major.mjs, check-empty-changeset.mjs, check:changeset-gate-self-tests and check:objectui-changeset are all green in the run above. check:nul-bytes is green, and both touched files were additionally swept by hand for control characters (grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]', no hits).

node scripts/pm/check-clause2-carriers.mjs --pair 16926 exits 0: "the clause-② declaration is readable in the fixed spelling and both carriers agree, and its diff carries no widening tell."

Every exit code above was captured before any pipe.

验收备注

Two observations, both noted, not filed, each with its successor named:

Nothing else was found. No dead code, naming or polish observations arose from this diff.


Generated by Claude Code

…ries its required name

The published README's namespace tour called `approvals.approve` / `approvals.reject`
with a bare comment string. Both declare
`(requestId: string, decision?: { actorId?; comment?; attachments? })`, so a TypeScript
reader gets TS2559 and a JavaScript reader gets nothing at all — the string goes out as
the request body where the route reads the decision object's fields, and the approval's
reason is silently dropped.

Type-checking the whole fence against the package's own built `dist/index.d.ts` turned up
one more call in the same defect class: `auth.register` was passed `{ email, password }`
while `RegisterRequestSchema` declares `name` as required, failing TS2345.

All three now match the spelling `content/docs/api/client-sdk.mdx` already carries. No
source changes; `packages/client/src/index.ts` was read only.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
@github-actions github-actions Bot added size/s documentation Improvements or additions to documentation tooling labels Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

⚠️ 1 changed file(s) yielded no anchor (packages/client/README.md), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files. Nothing else in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)).

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/client/README.md) — 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 — 14 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 9a89a0040de98fc77046c5b405db1a3935fd7b18packageMentionDocs.

Copy link
Copy Markdown
Collaborator Author

Accepted — CI is fully green on aaff32ce65, and the docs-drift gap is closed by measurement rather than by assertion

raw check-run rows 45
names carrying superseded earlier rows 6, each appearing 3× (Auto Label, Check PR Size, Check Changeset, No other open PR may claim the same single-writer path, Part-of PR must not also close its card, No other open PR may claim the same issue) — 12 superseded rows
distinct names after collapse 33
failure / cancelled / timed_out 0
queued / in_progress 0

⭐ This is the head where the collapse actually earns its keep: three separate workflow batches ran (the initial push, a re-trigger, and the body edit at 16:58), so six names carry three rows each. Reading the raw list uncounted would have triple-counted them. ⛔ And it changes no verdict — none of the 12 superseded rows is a red; the failure count is 0 in either direction.

Lint & Repo Gates finished last, at 17:09:59Z.

What I verified myself, at source

Read at head aaff32ce65 against merge-base ed6579b53b:

  • diff is exactly two filespackages/client/README.md (+3/−3) and one @objectstack/client patch changeset;
  • packages/client/src/index.ts is absent from the diff — read-only, as required while fix(client): organizations.getActiveMember addresses the organisation the caller NAMES, not whichever one the session has active #16761 holds it;
  • the approvals pair now reads { comment: 'LGTM' } / { comment: 'Incomplete' }, matching content/docs/api/client-sdk.mdx:359-360;
  • auth.register gains the name that RegisterRequestSchema declares required;
  • exactly one attribution block, in AGENTS.md's form;
  • node scripts/pm/check-clause2-carriers.mjs --pair 16926exit 0, re-run by me, and re-run again by the seat after the body edit.

⭐ The third failure is the reason the whole-fence instruction exists

The card reported two. Type-checking all 35 calls in the fence against the package's own built dist/index.d.ts found a third — auth.register({ email, password }), TS2345 — and it is the same defect class, so it was fixed here rather than filed. Shown RED first with the verbatim compiler output, then GREEN on the identical harness. ⛔ The zero on #16142's class (a documented method that no longer exists) is a reading rather than a silence, because the same harness fired three errors on the same text — those three are its control.

The docs-drift gap, closed properly

The bot declined to answer on this PR: "1 changed file(s) yielded no anchor (packages/client/README.md) … this is not a clean bill of health for those files." ⚠️ My dispatch word listed no docs-drift item at all — that omission was mine, not the delivering seat's, and I sent the requirement mid-flight.

It came back measured: the wrong-shape query returns 0 on content/ and 0 repo-wide, with two controls — corpus reach (the shape-agnostic form finds client-sdk.mdx:359-360) and regex liveness (the same wrong-shape pattern matches the pre-fix README at ed6579b53b). ⇒ The zero means no wrong shape, not dead grep. The PR body says plainly that the bot's non-answer was replaced by a hand sweep rather than treated as a pass.

And it reported a difference instead of reconciling it: I read four CHANGELOGs carrying the elided (requestId, …) form, the seat read five. The substance agrees (an elided argument asserts no shape either way) and the delta is a file count in a corpus that grows per release, taken on two different commits. Five is the number on the tree this PR delivers. That is exactly the right handling of a disagreement between two readings.

Recorded, not folded in

#16932, filed from this PR's out-of-scope finding: os register prompts "Name (optional)" and can send a payload without name, which RegisterRequestSchema:96 declares required — held open by an as any at register.ts:128, the same TS2345 this PR just fixed one carrier over. ⚠️ Whether the live better-auth route rejects a missing name is not measured, and the card says so and names the two fixes that follow from the two answers.

The check:skill-examples observation stays noted, not filed against #16209, correctly — that card owns "this fence is ungraded".

Arming

Ready-for-review first, auto-merge second. ⚠️ Auto-merge records merge_method: merge regardless; inert here, the queue squashes. ⛔ I neither approve nor merge.


Generated by Claude Code

@os-project-manager
os-project-manager marked this pull request as ready for review September 8, 2026 17:11
@os-project-manager
os-project-manager added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit 31e7542 Sep 8, 2026
47 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-16210-readme-approvals-decision-object branch September 8, 2026 17:36
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

2 participants