Skip to content

feat(client)!: converge analytics.query / meta / explain and automation.trigger on unwrapResponse — one SDK calling convention (#13079) - #14526

Draft
os-trump wants to merge 5 commits into
mainfrom
claude/issue-13079-sdk-envelope-convergence
Draft

feat(client)!: converge analytics.query / meta / explain and automation.trigger on unwrapResponse — one SDK calling convention (#13079)#14526
os-trump wants to merge 5 commits into
mainfrom
claude/issue-13079-sdk-envelope-convergence

Conversation

@os-trump

@os-trump os-trump commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Fixes #13079

Executes the maintainer's ruling on the card (2026-08-31, comment 5479237368, verbatim and untranslated): 「裁决:A,cloud 未测量照裁」 — 「四方法(analytics.query / analytics.meta / analytics.explain / automation.trigger)收敛 unwrapResponse,SDK 一套读法。」 Ruling items honoured: (1) analytics.queryDataset protected — its method body is byte-identical to base (sha1 2e6ec790…, 467 bytes; only its docblock says why it stays on res.json()); (2) migration note + breaking changeset; the 13 loud in-repo pins PR #13647 measured are this diff; (3) objectui's tolerant chain is objectui#7028's, time-gated behind this landing — not touched here; (4) cloud stays NOT MEASURED: CLOUD_CENSUS_COMMAND is kept in the census file, reworded to post-convergence semantics; (5) clause ② YES — needs:contract-review applied at creation. Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza

Contract change — per method, before and after

unwrapResponse never throws. Every non-2xx answer is thrown by ObjectStackClient.fetch BEFORE either reader runs, with the ADR-0112 envelope on the error (err.code, err.httpStatus, err.message, err.details) — that was true before this PR and is true after it. A 2xx body with a boolean success and a data key now resolves to data; a 2xx body with no data key resolves unchanged (pass-through). No dispatcher door behind these four routes sends a 2xx without data (a failed run has been a thrown 400 since #9378), so at the ENVELOPE level a resolved { success: false, error } is not a value a caller receives from them. At the PAYLOAD level one door differs: client.automation.trigger can resolve to an AutomationResult whose own success is false (with error set) — a run that classifyFlowRefusal (packages/runtime/src/flow-dispatch-status.ts) does not classify as 400 FLOW_FAILED / 409 FLOW_DISABLED / 422 FLOW_NO_START_NODE is answered 200 through deps.success(result) in respondToFlowTrigger (packages/runtime/src/domains/automation.ts), exactly as client.automation.execute already does for the same handler; before this PR that run reached the caller as { success: true, data: { success: false, error } }. Consequence for a caller: on trigger, r.success and r.error compile before AND after (both declared on AutomationResult, packages/spec/src/contracts/automation-service.ts), but their meaning moves from the envelope's (success always true on a resolved call, error never set on a 2xx) to the run's own — a branch on either must be re-read by hand, the compiler will not point at it. The three analytics methods have no such overlap: .data is TS2339 and nothing else compiles on both sides.

method route accepted 2xx — resolved BEFORE accepted 2xx — resolves NOW refused (non-2xx) — before AND after
analytics.query(q) POST /analytics/query { success: true, data: AnalyticsResult, meta? } — declared BaseResponse & { data: AnalyticsResult } AnalyticsResult (rows, fields, sql?, totals?) throws; e.g. 400 VALIDATION_ERROR on a refused query
analytics.meta(cube?) GET /analytics/meta AnalyticsMetadataResponse{ success, data: CubeMeta[], meta? } AnalyticsMetadataResponse['data'] — the bare cube list throws
analytics.explain(q) POST /analytics/sql AnalyticsSqlResponse{ success, data: { sql, params }, meta? } AnalyticsSqlResponse['data']{ sql, params } throws
automation.trigger(name, payload) POST /automation/trigger/:name { success: true, data: AutomationResult, meta? } — declared BaseResponse & { data: AutomationResult } AutomationResult — the same value automation.execute answers for the same handler (its own success may be false on an unclassified 200 refusal) throws: 400 FLOW_FAILED (#9378), 409 FLOW_DISABLED / 422 FLOW_NO_START_NODE (#9415), 404 unknown flow — unchanged
analytics.queryDataset(...) — PROTECTED POST /analytics/dataset/query (@objectstack/rest, bare) AnalyticsResult AnalyticsResult — unchanged, still res.json() throws — unchanged

What a caller loses: the envelope's success flag — always true on a resolved call — is no longer on the resolved value of these four; its meta slot is gone too, but these doors never populated it (each answers deps.success(result) with no meta argument, and JSON drops the undefined). Neither key was ever on any other SDK method's value. What a caller gains: one reading rule; trigger and execute stop answering two shapes for one handler. The wire is untouched: every route answers exactly the body it answered before, so raw-HTTP callers are unaffected.

Diff

  • packages/client/src/index.ts — the four methods end return this.unwrapResponse(res); return declarations move to the payload types, derived from the existing envelope types where the spec transcribes them (AnalyticsMetadataResponse['data'], AnalyticsSqlResponse['data']) and to the producer contracts otherwise (AnalyticsResult, AutomationResult) — no packages/spec edit. The BaseResponse import goes (unused). Docblocks rewritten so nothing documents the envelope read; queryDataset's docblock says why it stays.
  • packages/client/src/envelope-convergence.test.ts (new) — mocked transport: each of the four resolves to data with the envelope keys absent; exactly-once strip on a payload carrying its own success; the rejection path per door (400 FLOW_FAILED, 409 FLOW_DISABLED, 400 VALIDATION_ERROR) unchanged; the 2xx no-data pass-through pinned; queryDataset resolves to the bare body. RED on the base in exactly its five payload cases (the five failure-path/protected cases pass there, by design).
  • packages/client/src/return-type-precision.test.ts — the A sixth client-SDK erasure spelling, larger than the other five combined: 43 exported methods return res.json() directly, whose lib.dom type is Promise< any > #12104 pins REVERSED, not deleted: expectTypeOf binds the payload types; the four @ts-expect-error suppressions move from the payload read to the .data read. RED on the base as 4×TS2344 + 4×TS2578.
  • packages/client/src/analytics-automation-json-erasure.test.ts — the four cases re-driven against the real AnalyticsService / AutomationEngine / HttpDispatcher to the payload; the premise case now pins "dispatcher wraps once, SDK strips once" (meta() equals the dispatcher body's data).
  • packages/client/src/envelope-caller-census.test.ts — the derived ledger regenerated to the post-convergence world: ENVELOPE_DEPENDENT ratcheted at ZERO, new PAYLOAD_DEPENDENT verdict (18), RESULT_INSENSITIVE 10, NOT_SDK 1, 28 SDK sites; section 4 reads off the SDK source (comment-masked, namespace-anchored, sliced to the next sibling property) that the four end in unwrapResponse and queryDataset still dials getRoute('analytics') + ${route}/dataset/query and reads res.json(); the split-call positive control moves from 2 to 5; CLOUD_CENSUS_COMMAND kept, reworded (a .data read there is now a runtime break); objectui block names objectui#7028 as the tightening owner.
  • packages/client/src/client.test.ts — one stale comment (the six RESULT_INSENSITIVE sites there are untouched).
  • .changeset/client-envelope-convergence.md@objectstack/client: minor under the lockstep launch-window convention (check-changeset-no-major.mjs: the repo never writes major; **BREAKING** + feat(client)!: are the carriers), with the per-method migration table, the failure-path statement, and the ADR-0087 marker. Patch round after the contract review: the failure-path claims are scoped to the envelope level, trigger's own success/error are excepted from the compile-error claim, and the meta sentence no longer implies these doors populated it — prose only, no code change.

Out of scope and untouched: the ~40 other return res.json() sites in index.ts (the better-auth-backed auth.* / organizations.* / oauth.* families, a different class: REST-served bare bodies, not dispatcher envelopes). content/docs/api/client-sdk.mdx:312–329,433 and data-api.mdx:443 mention the four; none shows a .data read on the SDK value, so no devx edit is strictly required — reported for the PM, not edited. packages/client/README.md:273–277 likewise.

⚠ Known red, recorded rather than worked around: check:adr-0087-registration

The changeset states not-required (runtime-interface-only packages/client/src/index.ts#ObjectStackClient) and the gate refuses it (node scripts/check-adr-0087-registration.mjs --base 1dcb995f23 → exit 1): runtime-interface-only inherits the prescription refusal (#8299) and the body carries the ## Migration table the ruling requires; type-surface-only (#13080) is scoped by its predicate 4 to symbols that were any at the merge base, and these four were concrete envelope types there; registered would put an SDK reader change into a ledger that serves objectstack migrate meta. No verifiable category fits a concrete-to-concrete published runtime-interface move that ships its prescription. Filed as #14502 (spec-tooling); precedent for landing with the refusal recorded in-body: .changeset/adr0006-d2-client-environments-namespace.md. The changeset-check job (pr-automation.yml) is advisory, not one of the six required contexts.

Verification

Readings are quoted at the sha they were taken; exit codes were captured before any pipe; every build/test that acquired the shared verify lock (scripts/pm/os-verify-lock.sh) ended with VERDICT command-exit 0 on the wrapper. The patch-round commit changes only .changeset/client-envelope-convergence.md (prose), so every reading below stands for the code at the new head.

  • Base reading at 1dcb995f23 (untouched tree, dependency closure freshly built — 103 packages): census + producer-backed suite → Test Files 2 passed (2) · Tests 20 passed (20). The Census the caller population of the four envelope-returning SDK methods (#13079) #13647 ledger (13 ENVELOPE_DEPENDENT / 6 / 1, 19 SDK sites) holds on this base.
  • Red first at 1dcb995f23, only the new suite and the reversed pins present, index.ts unconverted: envelope-convergence.test.tsTests 5 failed | 5 passed (10) — exactly the five payload cases red; tsc --noEmit -p tsconfig.test.json → 17 errors: 4×TS2344 + 4×TS2578 in return-type-precision.test.ts, 9 payload reads in the new suite.
  • Converted, at 897585a74b: pnpm --filter @objectstack/client build exit 0; pnpm --filter @objectstack/client typecheck exit 0 (check:test-typecheck: OK — 0 file(s) / 0 error(s)); check:exported-any-returnsno NEW exported callable of @objectstack/client resolves to any: 317 callables reached; full client suite → Tests 1 failed | 430 passed (431), the one red being the census's own queryDataset source-slicer (a test-file defect, fixed over 5ce08b0f0c / 8f9001c665 / 9e5a10ee5c; packages/client/src/index.ts is blob 3575b137… in all four commits).
  • At 8f9001c665 (lock acquired): the four client suites → Tests 1 failed | 32 passed (33), the one red being the census assertion that matched the URL by its docblock spelling; tsc over the test program → 0 errors. 9e5a10ee5c changes only that assertion (getRoute('analytics') + ${route}/dataset/query, the code spelling); its exact regexes were evaluated by node against the final head's index.ts (all four predicates true; unwrapResponse absent) and the slicer's five slices are byte-exact (queryDataset = 467 bytes). The final-head vitest re-run of the census file is declared NOT RE-RUN locally — eight lock attempts over ~65 minutes never acquired (holders: issue-14336 runB.sh at 1000+ s, issue-13504, issue-13440); the node evaluation is the same predicate the test evaluates, and Test Core (required) runs the file on the PR.
  • Code head 9e5a10ee5c, gate union (dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, re-derived at this sha; 36 families, identical to the lists derived at 5ce08b0f0c and 8f9001c665): 31 families exit 0 — including check:query-options-erasure (ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new) and check:type-check-coverage; check:adr-0087-registration exit 1 = the recorded refusal above (check-adr-0087-registration has no verifiable disposition for a published runtime-interface change that was concrete at base and ships a code prescription — every category refuses #13079's changeset #14502); check-test-completeness exit 3 (PREREQUISITE NOT MET — this gate grades a saved turbo run test log) NOT MEASURED; pm/check-half-states exit 3 (PREREQUISITE NOT MET — repo-scoped reads are refused) NOT MEASURED; check:dual-build-cjs-loads exit 3 (This is NOT a pass: nothing was measured, 30 packages unbuilt here) NOT MEASURED. Whole-repo pnpm lint (eslint . --no-inline-config) exit 0 at 9e5a10ee5c; pnpm check:nul-bytes exit 0. At the patch-round head both changeset gates re-run: check-changeset-no-major green (This diff introduces no major bump), check-adr-0087-registration the same recorded refusal.
  • NOT MEASURED locally, by declared narrowing (same lock contention; CI's required jobs measure each): check:skill-examples (needs @objectstack/client-react built — verdict line packages/client-react/dist holds no .d.ts declarations — build first) and check:type-check-debt (a tsc re-measure over an unbuilt closure); and the DOWNSTREAM consumer typecheck (turbo run typecheck --filter='...@objectstack/client', prefix dots = consumers, ^build first). The narrowing's evidence: population = the six consumers pnpm names (cli, client-react, example-todo, example-crm, example-showcase, dogfood); a comment-masked, newline-tolerant, receiver-aware scan of their 723 source files finds 0 call-shaped occurrences of the four methods (positive control: the same scanner finds 29 in packages/client/src), the census §3 asserts zero production sites workspace-wide at this head, and the only declarations this diff moves are the four methods' return types plus an unused type import — so no consumer tsc program's verdict can move. TypeScript Type Check (required) runs it on the PR.

Ablation (reverse verification)

At 5ce08b0f0c (index.ts blob 3575b137…, identical at the final head). automation.trigger's conversion reverted on disk — annotation back to Promise< { success: boolean; data: AutomationResult } >, return this.unwrapResponse< AutomationResult >(res) back to return res.json() — by exact-string replacement asserting one occurrence each. Mutation proven on disk: probe counts 1 → 0 (the trigger unwrap line) and 44 → 45 (return res.json(); lines), blob 3575b137…add6b550… (the leg aborts if the blob had not moved). Direction predicted before running: RED, more diagnostics. Observed: convergence suite 2 failed (trigger payload; exactly-once strip), producer-backed suite 1 failed (trigger), census 1 failed (each of the four methods ends unwrapResponse) — Tests 5 failed | 26 passed (31) including the then-unfixed slicer case; tsc 8 errors: TS2344 (return-type-precision.test.ts:484), TS2578 (:503, the trigger .data suppression now unused), 3×TS2339 in the producer-backed suite, 3×TS2339 in the convergence suite. Restore by git checkout HEAD -- ABSOLUTE_PATH inside an EXIT INT TERM trap, proven by blob back to 3575b137… equal to HEAD:packages/client/src/index.ts, git diff HEAD --stat empty, git status --porcelain empty; the post-restore re-run at the same sha: convergence + producer-backed + precision green, tsc 0 errors. No rebuild leg was needed: vitest resolves ./index to src/index.ts and tsconfig.test.json reads src, so the mutation reached both suites without a build — the red itself is the evidence; dist/ (built once at 897585a74b) is read only by check:exported-any-returns, which was not part of the ablation.

…rigger on unwrapResponse

The four dispatcher-served methods ended `return res.json()` and handed their
callers the `{ success, data }` envelope whole, while every other
dispatcher-served method of ObjectStackClient strips it through
`unwrapResponse`. All four now end `return this.unwrapResponse(res)` and
declare the payload: `AnalyticsResult`, `AnalyticsMetadataResponse['data']`,
`AnalyticsSqlResponse['data']`, `AutomationResult` (the value
`automation.execute` already answered for the same handler).

`analytics.queryDataset` is protected: served bare by @objectstack/rest, it
keeps `res.json()`; its body is byte-identical (only its docblock says why).

Tests: `envelope-convergence.test.ts` (new, mocked transport: the payload on
all four, the unchanged rejection path, the no-`data` pass-through, and the
protected method); `analytics-automation-json-erasure.test.ts` re-driven
against the real producers to the payload; `return-type-precision.test.ts`
pins reversed (payload compiles, `.data` is the type error); the caller
census regenerated to the post-convergence ledger with the cloud census
command kept and reworded. One breaking changeset with the migration table.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
…t property

`queryDataset` is the last member of `analytics = {…}` and closes with a bare
`}`; the slicer required `},` and could not find it inside the block.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
…next sibling

The previous anchor (a method's own closing brace) stopped at queryDataset's
parameter type literal, and a `\s+` indentation capture absorbed newlines.
Comments are masked first so docblock prose cannot satisfy a code assertion.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
@github-actions github-actions Bot added the size/l label Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/client, touching 1 documentable anchor(s).

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

  • content/docs/api/client-sdk.mdx (via ObjectStackClient (symbol, a top-level class))
  • content/docs/api/environment-routing.mdx (via ObjectStackClient (symbol, a top-level class))
  • content/docs/api/wire-format.mdx (via ObjectStackClient (symbol, a top-level class))
  • content/docs/kernel/runtime-services/data-service.mdx (via ObjectStackClient (symbol, a top-level class))
  • content/docs/kernel/runtime-services/storage-service.mdx (via ObjectStackClient (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx (via ObjectStackClient (symbol, a top-level class))
  • content/docs/plugins/packages.mdx (via ObjectStackClient (symbol, a top-level class))
  • content/docs/protocol/kernel/realtime-protocol.mdx (via ObjectStackClient (symbol, a top-level class))

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

  • content/docs/releases/v17.mdx (via ObjectStackClient (symbol, a top-level class))

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
  • 2 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.

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 3bca6a4affb756031becce6ff7d908dd959eb8eapackageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 3bca6a4affb756031becce6ff7d908dd959eb8ea

⚠️ 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 3bca6a4affb756031becce6ff7d908dd959eb8ea → 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 2, 2026
…he docblock's

`analytics/dataset/query` only ever appeared in prose; the method spells it
`getRoute('analytics')` + `${route}/dataset/query`, which is what a
comment-masked slice can see.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
… level

Contract review corrections, prose only. `client.automation.trigger` is
excepted from the universal compile-error claim: `r.success` and `r.error`
compile before and after (both declared on `AutomationResult`), and their
meaning moves from the envelope's to the run's own — a refusal the door does
not classify as 400/409/422 is answered 200 through `deps.success(result)`
(`respondToFlowTrigger`, classification in `flow-dispatch-status.ts`), as
`execute` already does. The "not a value you will receive" sentence is
scoped to the envelope level, and the `meta` sentence no longer implies the
four doors ever populated it.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
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/l tests tooling

Projects

None yet

2 participants